Added sequences support on pyopendds [#18] #20
Conversation
Add basic support for publisher know issue : Segfault after write() call Signed-off-by: David Pierret <[email protected]>
manage sequences as list
There was a problem hiding this comment.
So this is a partial review. I still need to review the changes in itl2py and _pyopendds.cpp and look into the issue below. It's getting late for me and I'd like to give you at least some feedback since I've taken so long to review what you've been doing.
The issue I mentioned is I'm getting a segfault as Python is exiting when I try to run the test on my laptop. That is might not be your fault because it's something I once thought might happen when the Python objects are being destroyed, but I'm still looking into it. However before the segfault is happening I can see it's printing out the message, so it's exciting to see that part works!
| @@ -19,12 +20,13 @@ def init_opendds(*args, | |||
| verbose). It is printed to stdout. | |||
| ''' | |||
|
|
|||
| args = list(args) | |||
| args = list(sys.argv[1:]) | |||
There was a problem hiding this comment.
I don't have a problem with it being possible, but this isn't opt-in anymore like it is in a C++ OpenDDS application, where you have to explicitly give TheParticipantFactoryWithArgs the program arguments. What do you think about making this conditional on keyword argument like from_argv?
tests/basic_test/publisher.py
Outdated
| publisher = domain.create_publisher() | ||
| writer = publisher.create_datawriter(topic) | ||
|
|
||
| # Wait for Publisher to Connect |
There was a problem hiding this comment.
| # Wait for Publisher to Connect | |
| # Wait for Subscriber to Connect |
| { | ||
| // TODO: Encode or Throw Unicode Error | ||
| PyObject* repr = PyObject_Repr(py); | ||
| PyObject* str = PyUnicode_AsEncodedString(repr, "utf-8", "~E~"); |
There was a problem hiding this comment.
itl2py could set an encoding other than UTF-8. Also this also needs to throw if it failed.
| PyObject* str = PyUnicode_AsEncodedString(repr, "utf-8", "~E~"); | |
| PyObject* str = PyUnicode_AsEncodedString(repr, encoding, "~E~"); | |
| if (!str) throw Exception(); |
Also what's "~E~" do? I don't see it in the documentation for this function.
There was a problem hiding this comment.
done, "E" is the default string in case of error. replaced by NULL
| // Wait for samples to be acknowledged | ||
| DDS::Duration_t timeout = { 30, 0 }; | ||
| if (writer_impl->wait_for_acknowledgments(timeout) != DDS::RETCODE_OK) { | ||
| throw Exception( | ||
| "wait_for_acknowledgments error : ", Errors::PyOpenDDS_Error()); | ||
| } |
There was a problem hiding this comment.
This should be a separate operation like it is in OpenDDS, because this could be a lengthy process depending on how much stuff is being sent and to how many peers.
There was a problem hiding this comment.
agree, it's needed as a separate function on our application too.
| PyObject* write(PyObject* pywriter, PyObject* pysample) | ||
| { | ||
| DDS::DataWriter* writer = get_capsule<DDS::DataWriter>(pywriter); | ||
| if (!writer) PyErr_SetString(PyExc_Exception, "writer is a NULL pointer"); |
There was a problem hiding this comment.
| if (!writer) PyErr_SetString(PyExc_Exception, "writer is a NULL pointer"); | |
| if (!writer) throw Exception(); |
So PyErr_SetString doesn't stop this function from running. All it does is set up an exception that the interrupter will throw when it has the chance. Also get_capsule is already setting PyErr_SetString, all we would have to do here is to stop running this function, which is the purpose of throwing Exception() (with no arguments).
Signed-off-by: David Pierret <[email protected]>
|
Closing in favor of #30 |
This PR add sequences support to pyopendds.
depends #19